Skip to content

[Local Catalog] POS Settings: integrate with catalog data and full sync functionality #16144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 15 commits into from
Sep 30, 2025
Merged

[Local Catalog] POS Settings: integrate with catalog data and full sync functionality #16144

merged 15 commits into from
Sep 30, 2025

Conversation

@jaclync
Copy link
Contributor

@jaclync jaclync commented Sep 18, 2025 •
edited
Loading

Part of WOOMOB-1100
Please review #16166 first, or together.

Description

This PR integrates the settings UI with catalog stats & sync functionality. The implementation includes:

  • POSSettingsLocalCatalogViewModel: A view model that handles the UI logic for displaying catalog info and triggering full sync from CTA tap
  • Test coverage: Unit tests for both the view model and service with proper date comparison handling due to different precision in the database

Steps to reproduce

  1. Log in to a store eligible for POS if needed
  2. Tap on the POS tab
  3. Tap the ellipsis menu > Settings > Catalog --> the catalog size, incremental sync date, full sync date should be accurate
  4. In wp-admin, add a product/variation
  5. Tap on Refresh catalog to start a full sync --> the spinner should be shown in the button. for a site with a small catalog, it should be rather quick. the catalog size & last full sync date should be updated after the sync completes

Testing information

I tested on a small site, and a large site and left the settings while the full sync was still ongoing.

Screenshots

Latest copy:

Simulator Screenshot - iPad (A16) - 2025-09-30 at 11 06 42
Simulator.Screen.Recording.-.iPad.A16.-.2025-09-18.at.16.29.12.mov
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@jaclync jaclync added type: task An internally driven task. status: feature-flagged Behind a feature flag. Milestone is not strongly held. feature: POS labels Sep 18, 2025
@jaclync jaclync added this to the 23.4 milestone Sep 18, 2025
@jaclync jaclync changed the title Add POS catalog sync settings functionality [Local Catalog] POS Settings: integrate with catalog data and full sync functionality Sep 18, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Sep 18, 2025 •
edited
Loading

App Icon📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

App NameWooCommerce iOS Prototype
Build Numberpr16144-6293042
Version23.3
Bundle IDcom.automattic.alpha.woocommerce
Commit6293042
Installation URL6rubgov2tf8d8
 Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

@jaclync
Fix crashes in MainTabBarControllerTests due to fatalError from acc…
9fe048d 
…essing `ServiceLocator.grdbManager` when the feature flag is disabled.
@jaclync jaclync requested a review from joshheald September 18, 2025 10:04
@jaclync
Copy link
Contributor Author

jaclync commented Sep 18, 2025

Hi @joshheald no rush reviewing this PR, or just leave initial comments first on a high level review if you get a chance. I will try breaking it into two PRs if you will be reviewing it later.

@jaclync jaclync mentioned this pull request Sep 23, 2025
Merged
1 task
@jaclync jaclync changed the base branch from trunk to feat/WOOMOB-1100-pos-sync-settings-yosemite September 23, 2025 05:56
joshheald
joshheald approved these changes Sep 29, 2025
View reviewed changes
Copy link
Contributor

@joshheald joshheald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks great, thanks for doing this.

I found that when I switched stores, the old numbers were used. I can't immediately see why, but it only happened if I was quick to open POS, and corrected itself after closing and re-opening POS. So there's some state sticking around between launches. The sync coordinator is suspect here, because it's held on the service locator... but it shouldn't really play a part in getting the numbers, so perhaps there's something else that's not reset when it should be.

Also, we use "Last incremental update" and "Last full sync" as titles, which is inconsistent. I think it would stand out less if we just said "Last update", which is clearer anyway, or "Last incremental sync", though I like that less.

Base automatically changed from feat/WOOMOB-1100-pos-sync-settings-yosemite to trunk September 30, 2025 02:08
@jaclync
Copy link
Contributor Author

jaclync commented Sep 30, 2025

I found that when I switched stores, the old numbers were used. I can't immediately see why, but it only happened if I was quick to open POS, and corrected itself after closing and re-opening POS. So there's some state sticking around between launches. The sync coordinator is suspect here, because it's held on the service locator... but it shouldn't really play a part in getting the numbers, so perhaps there's something else that's not reset when it should be.

I noticed this issue during testing as well, it was from a recent CIAB change p1758613264239289/1758605672.214159-slack-CGPNUU63E. Kiwi had a workaround p1758690950124649/1758605672.214159-slack-CGPNUU63E and I had a PR #16173 that refactors this part. I will merge after testing with the latest trunk merged in.

@jaclync
Copy link
Contributor Author

jaclync commented Sep 30, 2025

Also, we use "Last incremental update" and "Last full sync" as titles, which is inconsistent. I think it would stand out less if we just said "Last update", which is clearer anyway, or "Last incremental sync", though I like that less.

If I understood correctly, I made the following copy changes to show "sync" instead of "sync" in the UI:

  • "Last incremental update" → "Last update"
  • "Last full sync" → "Last full update" (to match the pattern)

Along with other changes to use "update" for consistency in aba8530. We don't have a design for the settings yet, can make more copy/UI updates later.

Simulator Screenshot - iPad (A16) - 2025-09-30 at 11 06 42

@jaclync jaclync enabled auto-merge September 30, 2025 03:11
@jaclync jaclync merged commit 07b5fa9 into trunk Sep 30, 2025
13 checks passed
@jaclync jaclync deleted the feat/WOOMOB-1100-pos-sync-settings-basic branch September 30, 2025 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshheald joshheald joshheald approved these changes

Assignees

No one assigned

Labels

feature: POS status: feature-flagged Behind a feature flag. Milestone is not strongly held. type: task An internally driven task.

Projects

None yet

Milestone

23.4

Development

Successfully merging this pull request may close these issues.

4 participants

@jaclync @wpmobilebot @joshheald